Skip to content

fix(mobile): move the thread sync indicator into the header - #5000

Closed
carTloyal123 wants to merge 1 commit into
pingdotgg:mainfrom
carTloyal123:fix/mobile-sync-indicator-header
Closed

fix(mobile): move the thread sync indicator into the header#5000
carTloyal123 wants to merge 1 commit into
pingdotgg:mainfrom
carTloyal123:fix/mobile-sync-indicator-header

Conversation

@carTloyal123

@carTloyal123 carTloyal123 commented Jul 30, 2026

Copy link
Copy Markdown

What changed

The "Syncing threads..." indicator flashed in and out above the thread list. There were two independent causes, and this fixes both.

1. It lived inside the scroll view

WorkspaceConnectionStatus was rendered as ListHeaderComponent — literally row 0 — in both HomeScreen and ThreadNavigationSidebar. Every sync-state flip mounted or unmounted a row, which flashed the indicator and reflowed every row beneath it.

It now renders as a header button instead. The Android floating overlay that duplicated it is gone too.

2. The signal itself strobes

Moving the UI alone would not have stopped the blinking. hasSynchronizingShell re-enters "synchronizing" on every websocket resubscribe and retries expected failures on a 250ms cycle, so even a healthy connection produces a stream of sub-second sync blips.

The raw state is now reduced to a tone (offline / error / disconnected / connecting / syncing / idle) and settled before it renders:

  • Transient tones must persist 400ms before appearing — longer than the retry cycle, so a reconnect that resolves on its own never reaches the header at all.
  • Once shown, they stay 900ms, so a sync that resolves immediately after crossing the threshold doesn't appear and vanish within a frame.
  • Real faults (offline, connection error, no ready environment) bypass both delays — delaying those would mean hiding a problem worth seeing.

The settling decision is a pure function (planWorkspaceSyncToneSettlement) so the timing behaviour is unit-tested rather than implicit in a hook.

Before / after

Captured on iOS Simulator (iPhone 17 Pro, iOS 26.5) against a local server with six seeded threads. Each pair is the same screen: connected and idle, then with the environment disconnected.

Idle (connected) Disconnected / syncing
Before
After

Before: the status becomes row 0 of the list. "Tidy up mobile header spacing" is pushed from y≈307 to y≈422 — every row below it shifts down, then snaps back when the state clears.

After: the status is an icon in the header capsule next to •••. The list does not move at all — the first row stays at y≈307 in both states.

Screen recordings of the same disconnect/reconnect cycle:

(Media lives on a separate branch so it stays out of this diff.)

Details worth noting

  • The button keeps its slot when idle — it renders an empty box of the same size — so neighbouring header controls never shift as sync state changes. Trading a little dead space for zero layout movement seemed right for the bug being fixed.
  • Wired into all three headers: the Android home header, the iOS native header items, and the split-view sidebar.
  • iOS native bar-button items can't host a spinner, so they fall back to an SF Symbol; the React headers show an ActivityIndicator.
  • optionsVersion had to be extended — native header factories are stabilized, so a tone captured inside one can't change the options signature on its own and the icon would never update.
  • Tapping still opens environment settings, unchanged.
  • The full-page empty state deliberately keeps its inline pill. With no threads on screen it's the only thing explaining why the page is blank, and there's nothing below it to reflow.

Verification

  • tsc --noEmit clean
  • vp lint clean on changed files
  • Full mobile suite green (587 tests, 13 new covering tone derivation and settling)
  • Manually verified on iOS Simulator as above

Branched off current main. Split from #4999 so the two unrelated fixes stay reviewable separately.

The "Syncing threads..." indicator flashed in and out above the thread
list. Two causes, both fixed here.

It lived inside the scroll view. WorkspaceConnectionStatus was rendered
as ListHeaderComponent (row 0) in HomeScreen and ThreadNavigationSidebar,
so every sync flip mounted or unmounted a row and reflowed everything
below it. It now renders as a header button instead, and the Android
floating overlay it duplicated is gone.

The signal itself strobes. hasSynchronizingShell re-enters
"synchronizing" on every websocket resubscribe and on a 250ms retry
cycle, so relocating the UI alone would have kept the blinking. The raw
state is now reduced to a tone (offline / error / disconnected /
connecting / syncing / idle) and settled before it renders: transient
tones must persist 400ms — longer than the retry cycle, so self-resolving
blips never appear — and stay 900ms once shown. Faults bypass both
delays, since delaying those would hide a problem worth seeing.

The button keeps its slot when idle (an empty box of the same size), so
neighbouring header controls never shift. Wired into the Android home
header, the iOS native header items, and the split-view sidebar. Tapping
still opens environment settings.

The full-page empty state keeps its inline pill on purpose: with no
threads on screen it is the only thing explaining why the page is blank,
and there is nothing below it to reflow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

The native iOS sidebar header shifts its filter and settings controls horizontally whenever the sync tone transitions between idle and any visible tone. When syncTone is idle, workspaceSyncToneSymbol returns null, and createSidebarHeaderItems omits the sync bar item entirely; when the tone changes to connecting, error, etc., the item reappears. This inserts and removes the leading navigation bar item on each sync-state flip, causing the same header-layout movement the PR is intended to eliminate. The React header avoids this by always rendering a fixed-size WorkspaceSyncStatusButton slot (an empty box when idle). Consider rendering a fixed native bar item for the idle tone as well, so the filter/settings controls stay in place.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx around line 39:

The native iOS sidebar header shifts its filter and settings controls horizontally whenever the sync tone transitions between `idle` and any visible tone. When `syncTone` is `idle`, `workspaceSyncToneSymbol` returns `null`, and `createSidebarHeaderItems` omits the sync bar item entirely; when the tone changes to `connecting`, `error`, etc., the item reappears. This inserts and removes the leading navigation bar item on each sync-state flip, causing the same header-layout movement the PR is intended to eliminate. The React header avoids this by always rendering a fixed-size `WorkspaceSyncStatusButton` slot (an empty box when idle). Consider rendering a fixed native bar item for the idle tone as well, so the filter/settings controls stay in place.

// React headers: each change re-enters navigation.setOptions.
const syncTone = useSettledWorkspaceSyncTone(props.catalogState);
const syncSymbol = workspaceSyncToneSymbol(syncTone);
const syncLabel = workspaceConnectionStatusLabel(props.catalogState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium home/HomeHeader.tsx:324

The native iOS sync button's accessibility label (syncLabel) is derived from raw props.catalogState, while its icon (syncSymbol) is derived from the settled syncTone. During the 900 ms minimum-visible hold after syncing completes, the button still displays the syncing icon but workspaceConnectionStatusLabel already returns "Not connected", so VoiceOver announces a status that contradicts the visible indicator. Derive the label from the same settled tone as the icon, or settle the label alongside it.

Also found in 2 other location(s)

apps/mobile/src/features/home/WorkspaceSyncStatusButton.tsx:70

The button's visible state comes from settled tone, but its accessibility label is recomputed from the raw props.state. During the 900 ms minimum-visible period after syncing has completed, the button still renders a spinner while workspaceConnectionStatusLabel commonly returns Not connected; analogous rapid transitions can label one settled status as another. Screen-reader users therefore hear a status that contradicts the rendered indicator. Derive the label from the settled tone or settle the label alongside it.

apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx:1040

syncLabel is derived from the raw catalogState while syncSymbol is derived from the settled tone. During the 900 ms minimum-visible hold after connecting/syncing resolves, the native header still displays the sync icon but its accessibility label has already changed (typically to &#34;Not connected&#34;), so VoiceOver announces a state that does not match the visible settled status. Derive the label from the same settled tone/state as the icon.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/home/HomeHeader.tsx around line 324:

The native iOS sync button's accessibility label (`syncLabel`) is derived from raw `props.catalogState`, while its icon (`syncSymbol`) is derived from the settled `syncTone`. During the 900 ms minimum-visible hold after syncing completes, the button still displays the syncing icon but `workspaceConnectionStatusLabel` already returns `"Not connected"`, so VoiceOver announces a status that contradicts the visible indicator. Derive the label from the same settled tone as the icon, or settle the label alongside it.

Also found in 2 other location(s):
- apps/mobile/src/features/home/WorkspaceSyncStatusButton.tsx:70 -- The button's visible state comes from settled `tone`, but its accessibility label is recomputed from the raw `props.state`. During the 900 ms minimum-visible period after syncing has completed, the button still renders a spinner while `workspaceConnectionStatusLabel` commonly returns `Not connected`; analogous rapid transitions can label one settled status as another. Screen-reader users therefore hear a status that contradicts the rendered indicator. Derive the label from the settled tone or settle the label alongside it.
- apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx:1040 -- `syncLabel` is derived from the raw `catalogState` while `syncSymbol` is derived from the settled tone. During the 900 ms minimum-visible hold after connecting/syncing resolves, the native header still displays the sync icon but its accessibility label has already changed (typically to `"Not connected"`), so VoiceOver announces a state that does not match the visible settled status. Derive the label from the same settled tone/state as the icon.

return;
}

if (settlement.kind === "apply") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium home/use-settled-workspace-sync-tone.ts:100

When a non-transient fault (offline/error/disconnected) replaces a shown spinner, holdUntilRef keeps the stale spinner deadline. If the fault later clears to idle before that deadline expires, planWorkspaceSyncToneSettlement reuses the obsolete hold and schedules the idle transition hundreds of milliseconds in the future — so the header keeps showing the already-resolved fault for up to 900 ms after recovery. Reset holdUntilRef to 0 when applying an immediate non-transient fault, since the minimum-visible hold only applies to transient tones.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/home/use-settled-workspace-sync-tone.ts around line 100:

When a non-transient fault (`offline`/`error`/`disconnected`) replaces a shown spinner, `holdUntilRef` keeps the stale spinner deadline. If the fault later clears to `idle` before that deadline expires, `planWorkspaceSyncToneSettlement` reuses the obsolete hold and schedules the `idle` transition hundreds of milliseconds in the future — so the header keeps showing the already-resolved fault for up to 900 ms after recovery. Reset `holdUntilRef` to 0 when applying an immediate non-transient fault, since the minimum-visible hold only applies to transient tones.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06a58d9e-f326-4ed2-b04f-7168ea66bd96

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6330f6b. Configure here.

if (state.hasPendingShellSnapshot) return "syncing";
if (state.hasLoadedShellSnapshot && !state.hasReadyEnvironment) return "disconnected";
return "idle";
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconnect misclassified as error tone

High Severity

workspaceSyncTone returns error whenever connectionError is set, before it checks for an in-progress reconnect. Reconnecting environments normally carry lastFailure as connectionError, so active reconnects become a non-transient error triangle instead of a settled connecting spinner, and the label still says reconnecting.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6330f6b. Configure here.

const remainingHold = input.holdUntil - input.now;
return remainingHold <= 0
? { kind: "apply", tone: "idle" }
: { kind: "schedule", tone: "idle", delayMs: remainingHold };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fault clear delayed by stale hold

Medium Severity

Clearing to idle always respects holdUntil, even when the visible tone is already a real fault. After a spinner sets that timestamp, a later offline/error/disconnected indicator can linger for the leftover hold once the fault resolves, instead of dismissing immediately.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6330f6b. Configure here.

@macroscopeapp

macroscopeapp Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

3 blocking correctness issues found. Multiple unresolved review comments identify substantive bugs: a high-severity logic error where reconnects are misclassified as error states, accessibility label mismatches with the visual indicator, and layout shift issues on native iOS. These issues should be addressed before merging.

You can customize Macroscope's approvability policy. Learn more.

@carTloyal123

Copy link
Copy Markdown
Author

Superseded by #5023, which fixes the same bug with a better approach.

This PR moved the status into a header icon button. That removed the flash, but traded away the full status text — you lost "Reconnecting to " in favour of an ambiguous glyph, and a static sync icon does not read as "data is moving".

#5023 keeps the original status bar and its wording, and instead pins it above the list so it can no longer mount/unmount and reflow the rows. At rest it reports Synced N environments at <time> rather than disappearing, so the bar is a standing answer instead of something you only notice mid-sync.

Closing rather than force-pushing, since it is a different approach rather than an iteration on this one.

@carTloyal123
carTloyal123 deleted the fix/mobile-sync-indicator-header branch July 30, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant